Skip to content

chore(vm): optimize code style#6552

Open
anderson09-geek wants to merge 1 commit intotronprotocol:developfrom
anderson09-geek:optimize-code-style
Open

chore(vm): optimize code style#6552
anderson09-geek wants to merge 1 commit intotronprotocol:developfrom
anderson09-geek:optimize-code-style

Conversation

@anderson09-geek
Copy link

What does this PR do?

Why are these changes required?

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

DataWord amountArrayOffset = stack.get(stack.size() - 2);
DataWord witnessArrayLength = stack.get(stack.size() - 3).clone();
DataWord witnessArrayOffset = stack.get(stack.size() - 4);
BigInteger amountArrayLength = stack.get(stack.size() - 1).value();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it was changed from DataWord to BigInteger;
And the PR title doesn't match the code format modifications, so relevant details should be supplemented.

DataWord amountArrayOffset = stack.get(stack.size() - 2);
DataWord witnessArrayLength = stack.get(stack.size() - 3).clone();
DataWord witnessArrayOffset = stack.get(stack.size() - 4);
BigInteger amountArrayLength = stack.get(stack.size() - 1).value();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inlining memNeeded() is fine for readability, but can you confirm it didn't contain any boundary checks we're now missing. Also, verify that BigInteger produces identical results to DataWord for edge cases (very large value), since their overflow behavior differs.

I also think this PR need more relavant details for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants